Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complex impedance fixes #429

Merged
merged 7 commits into from
Jul 17, 2024
Merged

Complex impedance fixes #429

merged 7 commits into from
Jul 17, 2024

Conversation

dpdutcher
Copy link
Collaborator

The complex_impedance module has developed a number of bugs, e.g. #428, and this is my attempt to fix them.
Draft PR until able to test.

Copy link
Contributor

@msilvafe msilvafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't used this module in ages. Assuming you have tested with the updated changes (have you?), I'm happy to have them merged.

(ahh didn't realize this was draft...so I assume you're still testing but my statement still holds minimal flow down issues as this isn't used at site so if you test at pton and get it working happy to get it merged in)

@dpdutcher
Copy link
Collaborator Author

Whoops, commit message for f0083c7 should have been "Set run_analysis default True for take_complex_impedance_ob_sc"

@dpdutcher dpdutcher marked this pull request as ready for review July 15, 2024 23:33
Copy link
Collaborator

@jlashner jlashner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me, thanks for the fixes!

Since the default flux ramp rate is 4kHz so we can only get half of that.
@dpdutcher dpdutcher merged commit 26b1e7a into master Jul 17, 2024
1 check passed
@dpdutcher dpdutcher deleted the complex_impedance_fixes branch July 17, 2024 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants